Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Malicious site protection feature flags #3719

Conversation

alessandroboron
Copy link
Contributor

Task/Issue URL: https://app.asana.com/0/72649045549333/1207944134334659/f
Tech Design URL: https://app.asana.com/0/1206329551987282/1207273224076495/f
CC: @not-a-rootkit

Description:

Add Feature flag class as per tech design to check if malicious site protection feature is enabled and should check protection for domain based on the domain privacy preferences.

Steps to test this PR:

  1. Ensure unit tests pass

Definition of Done (Internal Only):

Copy Testing:

  • Use of correct apostrophes in new copy, ie rather than '

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 15
  • iOS 16
  • iOS 17

Theme Testing:

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch 3 times, most recently from 899ad16 to f156fd5 Compare December 11, 2024 17:16
sut = MaliciousSiteProtectionFeatureFlags(featureFlagger: featureFlaggerMock, privacyConfigManager: configurationManagerMock)
}

deinit {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that I think of nilling properties in deinit is probably not needed as it will do it automatically. I will fix it.

@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-address-bar-and-privacy-dashboard branch from 95a56b5 to 014ea13 Compare December 19, 2024 12:46
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from a41d843 to 99e6543 Compare December 19, 2024 12:48
Copy link

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

@github-actions github-actions bot added the stale label Dec 27, 2024
Copy link

github-actions bot commented Jan 4, 2025

This PR has been closed after 14 days of inactivity. Feel free to reopen it if you plan to continue working on it or have further discussions.

@github-actions github-actions bot closed this Jan 4, 2025
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-address-bar-and-privacy-dashboard branch from 014ea13 to b4d4dd0 Compare January 9, 2025 02:36
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from 99e6543 to a76c78f Compare January 9, 2025 02:42
@alessandroboron alessandroboron removed the request for review from jaceklyp January 9, 2025 03:36
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from a76c78f to 560ef30 Compare January 9, 2025 04:29
@@ -146,6 +150,8 @@ extension FeatureFlag: FeatureFlagDescribing {
return .remoteReleasable(.subfeature(PrivacyProSubfeature.isLaunchedROWOverride))
case .freeTrials:
return .remoteDevelopment(.subfeature(PrivacyProSubfeature.freeTrials))
case .maliciousSiteProtection:
return .remoteDevelopment(.subfeature(MaliciousSiteProtectionSubfeature.onByDefault))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subfeature required to enable remote rollout at a later stage.

@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from 7d3a83b to 744d6aa Compare January 10, 2025 07:19
Copy link
Contributor

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-address-bar-and-privacy-dashboard branch from bfe728f to 5e0082f Compare January 14, 2025 03:12
Base automatically changed from alessandro/malicious-site-protection-address-bar-and-privacy-dashboard to alessandro/malicious-site-protection January 14, 2025 03:25
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from 744d6aa to e791a06 Compare January 14, 2025 03:43
@alessandroboron alessandroboron merged commit 3af5452 into alessandro/malicious-site-protection Jan 14, 2025
13 checks passed
@alessandroboron alessandroboron deleted the alessandro/malicious-site-protection-feature-flags branch January 14, 2025 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants